added the solution of task named 'cache-decorator'#2181
added the solution of task named 'cache-decorator'#2181vovikLXUS wants to merge 6 commits intomate-academy:masterfrom
Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Great job on implementing the core logic of the cache decorator! The use of a closure to manage separate caches for each function is well done.
However, changes are needed for the way you handle mutable arguments. Your current implementation has a critical issue:
- The check for mutable data types returns a string (
"Cannot work with mutable data types"). This is problematic because the code calling the decorated function will receive a string instead of its expected return type, which will likely causeTypeErrors later in the program.
The task only requires the decorator to be intended for immutable arguments. The best approach here is to remove this custom check. Python will automatically raise a TypeError if an unhashable type (like a list) is used in the cache key, which is the standard and more informative way to handle this.
Please remove the manual check and let Python's default behavior manage unhashable types. Also, please address the flake8 style issues reported in the check logs to clean up the code formatting.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Important
Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.
Tip
If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.
Caution
Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.
Footnotes
| if isinstance(kwargs.values(), (list, dict, set)): | ||
| return "Cannot work with mutable data types" |
There was a problem hiding this comment.
This check for mutable data types has a few issues:
- It doesn't work as intended because
kwargs.values()returns adict_valuesobject, soisinstancewill always beFalse. - It only checks keyword arguments (
kwargs) and not positional arguments (args). - Returning a string can cause type errors for the code that calls the decorated function.
The task description states this decorator is intended for functions with immutable arguments. Python will automatically raise a TypeError if you try to use an unhashable type (like a list) as part of a dictionary key, which is more informative. It's better to remove this custom check and let Python handle the error.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Good work on the core caching logic! The use of a closure to manage separate caches for each function is implemented correctly.
However, this submission requires changes because the primary issue from the previous review has not been addressed. The manual check for mutable arguments is still present and must be removed. Your code should not return a custom error string like "Sorry, I can't cache mutable types".
The correct and standard approach is to remove this check entirely and allow Python's default behavior to take over. When an unhashable (mutable) type like a list is used as a dictionary key, Python will automatically raise a TypeError, which is the expected outcome for this task. Please remove the if block that performs this manual check.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Important
Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.
Tip
If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.
Caution
Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.
Footnotes
| if isinstance(kwargs.values(), (list, dict, set)): | ||
| return "Cannot work with mutable data types" |
There was a problem hiding this comment.
This manual check for mutable types should be removed. As mentioned in the previous review, returning a string can cause unexpected errors for the function's caller. The standard approach is to let Python raise a TypeError if an unhashable (mutable) argument is used, which will happen automatically when creating the key on line 13.
Also, this check is not implemented correctly: isinstance on kwargs.values() will not work as you expect, and it completely ignores mutable types passed in *args.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Excellent work on this task! Your implementation of the cache decorator is spot-on. You've correctly removed the manual checks for mutable arguments and are now relying on Python's built-in TypeError, which is exactly what was needed.
Your solution's logic is perfect, but the automated style checker found a few minor formatting issues. I am approving your solution as an educational exception so you can move forward, but please take a moment to review these points for future reference:
app/main.py:7:5: It's best practice to have one blank line before a nested function definition.app/main.py:9:5: There shouldn't be a blank line between a decorator and the function it decorates.app/main.py:16:1: A blank line contains unnecessary whitespace.
Fixing these small details will make your code even cleaner and more professional. Great job overall!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Important
Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.
Tip
If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.
Caution
Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.
Footnotes
vsmutok
left a comment
vsmutok
left a comment
There was a problem hiding this comment.
Please fix the flake8 errors.
3 participants
No description provided.